-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix button opacity problem when saving personal details #10229
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM though I'll let @tgolen chime in since he was looking at this more closely
Merging since this fix is pretty useful, still interested in feedback from @tgolen if you have any :D |
Fix button opacity problem when saving personal details (cherry picked from commit f6cb210)
…-10229 🍒 Cherry pick PR #10229 to staging 🍒
🚀 Cherry-picked to staging by @Beamanator in version: 1.1.88-4 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
cc @tgolen since you were also looking into this solution yesterday
Details
The problem was, as mentioned here, some rerendering or gestures made the Opacity animation stop before it finished. In that comment I show some console logs that indicate the Animation never finished, so in this fix I make sure the Undim animation will always finish. It's possible we'll need the same for Dim in the future, but that's not something we need to worry about right now.
One alternate solution I had was to try to check if the value of
this.opacity
was < 1 & ifthis.props.shouldDim
wasfalse
, like this:However this doesn't work because
this.opacity
is anAnimated
value - to get its value you have to do something hacky (see this SO)So instead of doing something hacky, we now just make sure it fully undims (becomes fully visible) when needed.
Fixed Issues
$ #10230
Tests
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)QA Steps
Screenshots
Web
Screen.Recording.2022-08-03.at.1.04.18.PM.mov
Mobile Web
Screen.Recording.2022-08-03.at.1.21.17.PM.mov
Desktop
iOS
Screen.Recording.2022-08-03.at.1.44.13.PM.mov
Android
Screen.Recording.2022-08-03.at.1.51.48.PM.mov